Repository navigation
(GH-1081) Integrate settings through CLI and lib - #1769
Mikey Lombardi (He/Him) (michaeltlombardi) wants to merge 2 commits into
Conversation
70decef to
1745076
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical defects can ignore policy settings, bypass executable-path restrictions, and permanently delete user preferences during tests.
9 open issues
Restore the fallback user settings file after tests · New Use the correct plural settings-file environment variable · New Expect one error record at error trace level · New Canonicalize and validate executable paths consistently · New Load external settings in the library initializer · New Propagate false for the settings-file override · New Allow discovery without PATH when environment paths are disabled · New Search configured resource directories for executables · New Preserve UNC paths during Windows canonicalization · New
What changed in this PR
Continues #1081 by connecting structured DSC settings to CLI startup, resource discovery, and command execution, replacing ad-hoc settings access.
Changes:
- Adds shared settings initialization and cached resource paths, with restricted-directory checks before command execution.
- Supports explicit
--ignore-settings-file=falseand resolves tracing through structured settings. - Expands settings integration tests, updates discovery expectations, and parameterizes CSP tests.
| File | Description |
|---|---|
lib/dsc-lib/src/util.rs |
Removes legacy settings loading and updates executable resolution. |
lib/dsc-lib/src/settings/mod.rs |
Exposes the global settings module. |
lib/dsc-lib/src/settings/global.rs |
Adds shared settings storage and accessors. |
lib/dsc-lib/src/settings/fields/tracing.rs |
Adds trace-level type conversions. |
lib/dsc-lib/src/settings/dsc_settings.rs |
Uses public setting names in warnings. |
lib/dsc-lib/src/dscresources/command_resource.rs |
Checks executable directories against resolved restrictions. |
lib/dsc-lib/src/dscerror.rs |
Adds a restricted-executable error. |
lib/dsc-lib/src/discovery/command_discovery.rs |
Uses structured settings and caches discovery paths. |
lib/dsc-lib/locales/en-us.toml |
Updates settings and restriction diagnostics. |
dsc/tests/other/dsc_settings.tests.ps1 |
Expands coverage for settings sources, precedence, and restrictions. |
dsc/tests/other/dsc_settings_policy_security.tests.ps1 |
Updates policy filenames and assertion diagnostics. |
dsc/tests/discovery/dsc_discovery.tests.ps1 |
Adjusts restricted-path and relative-executable tests. |
dsc/tests/discovery/dsc_adapter.tests.ps1 |
Accepts resolved executable paths in trace assertions. |
dsc/src/util.rs |
Configures tracing from resolved settings. |
dsc/src/main.rs |
Initializes settings and propagates selected values to children. |
dsc/src/args.rs |
Supports boolean flag overrides and typed CLI settings conversion. |
Cargo.lock |
Records CSP test dependencies. |
adapters/csp/tests/cli.rs |
Parameterizes tests and gates elevated Windows cases. |
adapters/csp/Cargo.toml |
Adds parameterized-test and elevation-check dependencies. |
Review effort: Balanced
| let Some(current_env_path) = env::var_os("PATH") else { | ||
| return Err(DscError::Operation(t!("discovery.commandDiscovery.failedGetEnvPath").to_string())); | ||
| }; | ||
|
|
||
| // if exe home is not already in PATH env var then add it to env var | ||
| // and list of searched paths | ||
| if let Some(exe_home) = get_exe_path()?.parent() { | ||
| let exe_home_pb = &exe_home.to_path_buf(); | ||
| let mut env_paths = env::split_paths(¤t_env_path).collect::<Vec<_>>(); |
There was a problem hiding this comment.
I'm not sure I agree with this recommendation. I can't think of a time when the PATH would be undefined for a functional system and even when append_env_path is false that only affects discovery, not invocation, unless restricted is true, which is covered in a separate logic branch.
This behavior also retains the prior behavior of failing when PATH can't be retrieved.
4e5ca84 to
ea5fd42
Compare
This change continues the work for using structured settings for DSC. This change: - Defines a globally accessible static for performantly accessing and using the resolved settings. - Replaces ad-hoc settings access with accessing the global static. - Updates the `--ignore-settings-file` argument so users can negate the flag and override the environment variable.
Prior to this change, some of the csp integration tests failed when run in a non-elevated context. This change: - Guards the test behavior on whether the current context is elevated. - Uses `test_case` to treat each test case in the tests as separate tests instead of failing the entire test when one case fails.
ea5fd42 to
9d76147
Compare


PR Summary
This change continues the work for using structured settings for DSC. This change:
--ignore-settings-fileargument so users can negate the flag and override the environment variable.PR Context
Continues the work from #1081